Skip to content

precompiles: Validate and preprocess expmod input #1195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 15, 2025

Conversation

chfast
Copy link
Member

@chfast chfast commented Apr 14, 2025

Decompose the expmod input into separate bytes_views for base, exp, mod. Handle arguments padding. Handle mod being zero.

Simplify and vastly reduce the expmod stub by using the preprocessed arguments and handling trivial cases before doing the result lookup.

@chfast chfast added precompiles Related to EVM precompiles has_dependencies PR depends on not merged yet PR labels Apr 14, 2025
@chfast chfast requested a review from rodiazet April 14, 2025 08:52
Copy link

codecov bot commented Apr 14, 2025

Codecov Report

Attention: Patch coverage is 97.43590% with 2 lines in your changes missing coverage. Please review.

Project coverage is 94.82%. Comparing base (1d574cf) to head (36e817f).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
test/state/precompiles_stubs.cpp 92.85% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1195      +/-   ##
==========================================
- Coverage   94.89%   94.82%   -0.08%     
==========================================
  Files         171      171              
  Lines       19687    19406     -281     
==========================================
- Hits        18682    18401     -281     
  Misses       1005     1005              
Flag Coverage Δ
eof_execution_spec_tests 2.57% <0.00%> (+0.03%) ⬆️
ethereum_tests 21.17% <58.97%> (-1.21%) ⬇️
ethereum_tests_silkpre 18.24% <20.96%> (+0.37%) ⬆️
execution_spec_tests 18.33% <58.97%> (-1.25%) ⬇️
unittests 92.48% <93.58%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
test/state/precompiles.cpp 99.05% <100.00%> (+0.05%) ⬆️
test/unittests/precompiles_expmod_test.cpp 100.00% <100.00%> (ø)
test/state/precompiles_stubs.cpp 93.10% <92.85%> (-6.32%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chfast chfast force-pushed the precompiles/expmod_input branch from f5fc6d3 to 8af3ec8 Compare April 14, 2025 15:59
Copy link
Contributor

@rodiazet rodiazet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall ok, one comment added.

@chfast chfast force-pushed the precompiles/fix_expmod_gas branch from 45b766f to f322609 Compare April 15, 2025 10:09
Base automatically changed from precompiles/fix_expmod_gas to master April 15, 2025 10:28
@chfast chfast force-pushed the precompiles/expmod_input branch from 8af3ec8 to e015ea5 Compare April 15, 2025 10:29
Decompose the expmod input into separate bytes_views for base, exp, mod.
Handle arguments padding. Handle mod being zero.

Simplify and vastly reduce the expmod stub by using the preprocessed
arguments and handling trivial cases before doing the result lookup.
@chfast chfast force-pushed the precompiles/expmod_input branch from e015ea5 to 36e817f Compare April 15, 2025 10:32
@chfast chfast enabled auto-merge (squash) April 15, 2025 10:45
@chfast chfast merged commit e3ff3b9 into master Apr 15, 2025
23 checks passed
@chfast chfast deleted the precompiles/expmod_input branch April 15, 2025 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has_dependencies PR depends on not merged yet PR precompiles Related to EVM precompiles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants